Skip to content

fix(observability): read T3CODE_OTEL_SDK_DISABLED before OTEL_SDK_DISABLED - #55

Merged
yordis merged 3 commits into
mainfrom
yordis/fix-otel-sdk-disabled-precedence
Sep 19, 2026
Merged

yordis merged 3 commits into
mainfrom
yordis/fix-otel-sdk-disabled-precedence

Conversation

@yordis

@yordis yordis commented Sep 19, 2026

Copy link
Copy Markdown
Member
  • Every other setting in the OTLP chain asks T3 Code's own name first and falls back to the standard one. Whether anything is exported at all was the exception: only OTEL_SDK_DISABLED could answer it, so the answer came from whatever the machine happened to inherit.
  • Asking ours first is what makes the setting recoverable. A profile that exports OTEL_SDK_DISABLED=true for every other SDK on the box used to leave no way to get T3 Code's telemetry back except unsetting a variable the rest of the machine depends on. T3CODE_OTEL_SDK_DISABLED=false is now that way.
  • Either name still stops every export, Settings included, because that is one setting with two sources rather than two switches with different reach.

An environment variable should not be able to countermand an endpoint the
user configured in Settings or named with T3CODE_OTLP_*, which is the rule
every other OTEL_* name already follows here. The kill switch a shared
machine needs is now T3CODE_OTEL_SDK_DISABLED, a name T3 Code owns.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Sep 19, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes global telemetry export on/off for server and desktop when ambient OTEL_SDK_DISABLED is set; behavior is intentional but operators must know the new override name.

Overview
Export on/off now follows the same T3 Code name first, then standard OTEL order as the rest of OTLP config. Shared otelEnvironment.load reads T3CODE_OTEL_SDK_DISABLED with T3-style booleans (true/1/yes/on and negatives); only when that name is unset or unreadable does OTEL_SDK_DISABLED apply (still spec-strict: only true disables).

T3CODE_OTEL_SDK_DISABLED=false can keep server and desktop exporting when a shell profile sets OTEL_SDK_DISABLED=true, without unsetting the profile. Disable messaging is centralized in otelEnvironment (including a hint to set the T3 override); duplicate warnings were removed from desktop OTLP resolution and the server observability layer.

Tests cover override, parsing, and end-to-end config/desktop resolution; ops and fork docs describe the new variable and precedence.

Reviewed by Cursor Bugbot for commit 8c9ae36. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Sep 19, 2026
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: TrogonStack/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 6a1e2d07-8d7a-41c6-bed2-adb522b5e6f9

📥 Commits

Reviewing files that changed from the base of the PR and between a6e60da and 8c9ae36.

📒 Files selected for processing (8)
  • apps/desktop/src/app/DesktopOtlpExport.test.ts
  • apps/desktop/src/app/DesktopOtlpExport.ts
  • apps/server/src/cli/config.test.ts
  • docs/fork/0018-the-standard-otel-variables-are-honored.md
  • docs/fork/0022-the-desktop-app-reports-its-own-work.md
  • docs/operations/observability.md
  • packages/shared/src/otelEnvironment.test.ts
  • packages/shared/src/otelEnvironment.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: TrogonStack/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c538792f-3645-4017-be2d-6d330416e80c

📥 Commits

Reviewing files that changed from the base of the PR and between d6d0b1d and a6e60da.

📒 Files selected for processing (10)
  • apps/desktop/src/app/DesktopOtlpExport.test.ts
  • apps/desktop/src/app/DesktopOtlpExport.ts
  • apps/server/src/cli/config.test.ts
  • apps/server/src/cli/config.ts
  • apps/server/src/observability/Layers/Observability.ts
  • docs/fork/0018-the-standard-otel-variables-are-honored.md
  • docs/fork/0022-the-desktop-app-reports-its-own-work.md
  • docs/operations/observability.md
  • packages/shared/src/otelEnvironment.test.ts
  • packages/shared/src/otelEnvironment.ts
💤 Files with no reviewable changes (1)
  • apps/server/src/observability/Layers/Observability.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds T3CODE_OTEL_SDK_DISABLED as a global telemetry shutdown switch. OTEL_SDK_DISABLED now disables only routes configured by standard variables. Server and desktop exporters, tests, warnings, and documentation reflect this separation.

Changes

Telemetry disablement controls

Layer / File(s) Summary
Shared environment controls
packages/shared/src/otelEnvironment.ts, packages/shared/src/otelEnvironment.test.ts
Adds forceDisabled, parses T3CODE_OTEL_SDK_DISABLED, emits distinct warnings, and covers affirmative and invalid values.
Server and desktop exporter integration
apps/server/src/cli/config.ts, apps/server/src/cli/config.test.ts, apps/server/src/observability/Layers/Observability.ts, apps/desktop/src/app/DesktopOtlpExport.ts, apps/desktop/src/app/DesktopOtlpExport.test.ts
Uses forceDisabled to clear all export endpoints. Standard disablement preserves named endpoints. Tests and warning behavior match the split.
Disablement documentation
docs/fork/0018-the-standard-otel-variables-are-honored.md, docs/fork/0022-the-desktop-app-reports-its-own-work.md, docs/operations/observability.md
Documents both switches, their scope, accepted values, precedence, warnings, and environment variable references.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant EnvironmentLoader
  participant ServerConfig
  participant DesktopOtlpExport
  EnvironmentLoader->>ServerConfig: provide forceDisabled and signal settings
  EnvironmentLoader->>DesktopOtlpExport: provide forceDisabled and signal settings
  ServerConfig->>ServerConfig: clear all OTLP URLs when forceDisabled is true
  DesktopOtlpExport->>DesktopOtlpExport: disable all signals when forceDisabled is true
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the observability change and the precedence between T3CODE_OTEL_SDK_DISABLED and OTEL_SDK_DISABLED.
Description check ✅ Passed The description clearly explains what changed and why. It covers the new application-owned switch and its relationship to OTEL_SDK_DISABLED. The template headings and checklist are omitted, but the ke…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 19, 2026

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −6 B (−0.0%) 15.1 KiB
Codex Thread snapshot wire 7.1 KiB 7.1 KiB −2 B (−0.0%) 7.3 KiB
Codex Live turn WebSocket wire 6.4 KiB 6.4 KiB −4 B (−0.1%) 7.8 KiB
Codex Live turn WebSocket decoded 56.2 KiB 56.2 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 9 9 0 (0.0%) 21
Claude Total thread wire 13.5 KiB 13.5 KiB −4 B (−0.0%) 15.1 KiB
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +4 B (+0.1%) 7.3 KiB
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB −8 B (−0.1%) 7.8 KiB
Claude Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB
Claude Live turn messages 9 9 0 (0.0%) 21

Baseline: d6d0b1d · PR result: 8c9ae36 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Scoping OTEL_SDK_DISABLED to its own route was the wrong reading of the
precedence rule. Precedence is about which name answers a setting, and
turning export off is one setting: T3CODE_OTEL_SDK_DISABLED answers it,
OTEL_SDK_DISABLED answers it when ours is unset. Asking ours first is
what a machine needs to keep T3 Code exporting while its profile
disables every other SDK.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis changed the title fix(observability): OTEL_SDK_DISABLED stops only the route it belongs to fix(observability): read T3CODE_OTEL_SDK_DISABLED before OTEL_SDK_DISABLED Sep 19, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit cf7ce5b into main Sep 19, 2026
20 checks passed
@yordis
yordis deleted the yordis/fix-otel-sdk-disabled-precedence branch September 19, 2026 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant